-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Waking the other side on async consumer / producer close #30
Conversation
let t0 = std::thread::spawn(move || { | ||
execute!(async { | ||
drop(prod); | ||
assert_eq!(stage.fetch_add(1, Ordering::SeqCst), 0); | ||
}); | ||
}); | ||
let t1 = std::thread::spawn(move || { | ||
execute!(async { | ||
cons.wait_occupied(1).await; | ||
assert_eq!(stage_clone.fetch_add(1, Ordering::SeqCst), 1); | ||
assert!(cons.is_closed()); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must separate as two polling, otherwise waking the wrong party also passes this test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good note, join!
-ing multiple futures can actually lead to falsely passing this test because of spurious wake-ups.
Also the usage of some async runtime (like Tokio or async-std) may be considered for testing. Each async
block then can be ran in separate task.
pub trait AsyncRingBuffer: RingBuffer + AsyncProducer + AsyncConsumer { | ||
fn wake_producer(&self); | ||
fn wake_consumer(&self); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add two methods for waking producer / consumer for trait AsyncRingBuffer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doubt that such low-level controls should be exposed to user but I can't suggest a better way to do this yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Just realized it's interesting that Rust does not actually have private trait fn.
Okay, the request looks good. I've also began to write test on this, but yours seem to be better. I'm merging it, thank you for your contribution! |
No description provided.